Skip to content

fix: isolate callId counter per sandbox for parallel test support#2715

Merged
fatso83 merged 2 commits into
sinonjs:mainfrom
singhvishalkr:fix-parallel-sandbox-callid
Jun 16, 2026
Merged

fix: isolate callId counter per sandbox for parallel test support#2715
fatso83 merged 2 commits into
sinonjs:mainfrom
singhvishalkr:fix-parallel-sandbox-callid

Conversation

@singhvishalkr

@singhvishalkr singhvishalkr commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

When tests run in parallel with separate sandboxes, the \calledImmediatelyBefore\ and \calledImmediatelyAfter\ assertions fail unexpectedly. This happens because proxy-invoke.js uses a global \callId\ counter shared across all sandboxes.

The fix introduces a per-sandbox context object that holds its own \callId\ counter. Each sandbox passes this context through the spy/stub/fake creation chain, and the proxy stores it for use during invocation.

Implementation:

  • proxy-invoke.js: Use \ his.sinonContext.callId\ instead of a global variable, with a default context for backward compatibility when used outside sandboxes
  • proxy.js: Accept optional context parameter and store it on the proxy
  • spy.js/stub.js/fake.js: Thread context through creation functions
  • sandbox.js: Create a sandbox-specific context and pass it to spy/stub/fake via new \withContext\ methods

Added tests that verify call order assertions work correctly across independent sandboxes.

The public API remains unchanged. Direct use of \sinon.spy()\ without a sandbox uses a shared default context (same behavior as before), while sandbox-created spies/stubs/fakes get isolated counters.

Fixes #2472

@fatso83

fatso83 commented May 20, 2026

Copy link
Copy Markdown
Contributor

The implementation looks good, but the tests are failing 🙃

@singhvishalkr
fix: isolate callId counter per sandbox for parallel test support
2b77b77 
The global callId counter in proxy-invoke.js caused calledImmediatelyBefore
and calledImmediatelyAfter to fail when tests run in parallel with separate
sandboxes. Each sandbox now maintains its own callId counter, passed through
the spy/stub/fake creation chain via a context object.

Fixes sinonjs#2472
@singhvishalkr singhvishalkr force-pushed the fix-parallel-sandbox-callid branch from 984b721 to 2b77b77 Compare June 11, 2026 17:48
@singhvishalkr

singhvishalkr commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Updated the branch.

The failing tests were from the sandbox wrappers using the array helper incorrectly when forwarding arguments into the context-aware spy/stub/fake constructors. That dropped the original stub target metadata, which broke accessor stubs and nested sandbox behavior. The wrappers now use the existing commons array helpers consistently, and src/sinon/spy.js has been formatted.

Verification:

  • npx prettier --check on changed files
  • npx eslint --max-warnings 0 on changed files
  • npx mocha -R spec test/src/sandbox-test.js --grep "call order isolation"
  • npx mocha -R dot test/src/sandbox-test.js --timeout 10000

I could not use npm run test-node on Windows because Rollup fails locally before tests with: Entry module "src/create-sinon-api.js" cannot be external. The source-level sandbox tests above cover the failing behavior directly.

@fatso83

fatso83 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor 
  1551 passing (2s)
  12 pending
  1 failing

  1) fake
       should reject non-Function argument:
     AssertionError: [assert.exception] Expected exception
      at Object.fail (node_modules/@sinonjs/referee/lib/create-fail.js:12:21)
      at Object.fail (node_modules/@sinonjs/referee/lib/define-assertion/index.js:57:17)
      at Object.assert (node_modules/@sinonjs/referee/lib/assertions/exception.js:44:21)
      at assertion (node_modules/@sinonjs/referee/lib/define-assertion/index.js:62:23)
      at referee.<computed>.<computed> [as exception] (node_modules/@sinonjs/referee/lib/define-assertion/index.js:111:22)
      at file:///home/runner/work/sinon/sinon/test/src/fake-test.js:94:20
      at Array.forEach (<anonymous>)
      at Context.<anonymous> (file:///home/runner/work/sinon/sinon/test/src/fake-test.js:93:18)
      at process.processImmediate (node:internal/timers:504:21)

@singhvishalkr

singhvishalkr commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in d216f3f9. The sandbox context plumbing now distinguishes an omitted fake() argument from an explicit fake(undefined), so the existing non-Function validation is preserved.

Ran:

  • npm.cmd exec -- mocha -R spec test/src/fake-test.js --grep "should reject non-Function argument"
  • npm.cmd exec -- mocha -R dot test/src/fake-test.js
  • npm.cmd exec -- mocha -R dot test/src/sandbox-test.js --grep "call order isolation"
  • npx.cmd prettier --check src/sinon/fake.js
  • npx.cmd eslint --max-warnings 0 src/sinon/fake.js
  • git diff --check

@fatso83 fatso83 merged commit 4db4fef into sinonjs:main Jun 16, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fatso83 fatso83 fatso83 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Sinon fails in comparing call order when running tests in parallel

2 participants

@singhvishalkr @fatso83